-
Notifications
You must be signed in to change notification settings - Fork 579
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cmd/snap-confine: introduce sc_invocation #6571
Conversation
Currently passing data around snap-confine is not very easy because of historically organic growth of the codebase. To simplify some of that I'd like to introduce sc_invocation. An ad-hoc structure that captures some of the variables that participate in the essential part of snap-confine's logic. The purpose of this patch is to introduce this structure, switch some of the code to use it (as opposed to accessing the local variables) and set the stage for a subsequent patch that moves a large chunk of code into the two new stubs: enter_{,non_}classic_execution_environment(). This should eventually allow us to make a single call to sc_should_use_normal_mode() and simply pass the result around via the invocation structure. There is plenty of more refactoring that can be done after this patch is in place. Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Handling uid/gid is a bit more delicate as those values routinely change thought execution. Instead of passing them around in the invocation structure pass them explicitly to the only place that needs it for now. Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Codecov Report
@@ Coverage Diff @@
## master #6571 +/- ##
=========================================
- Coverage 79.13% 79.1% -0.04%
=========================================
Files 580 580
Lines 45089 45089
=========================================
- Hits 35681 35667 -14
- Misses 6509 6521 +12
- Partials 2899 2901 +2
Continue to review full report at Codecov.
|
we discussed using sc_invocation only in the non-classic path for now |
After a good night's sleep I realised I should not have put it there in the first place. This will allow the invocatoin to collect and contain only the parts of the arguments that are related to the invocation of snap-confine and leave out certain things that have more complex state (uids) or are an implementation detail (apparmor struct) out of it. Signed-off-by: Zygmunt Krynicki <me@zygoon.pl>
…nvironment The classic path is (for now) empty so let's not use the invocation there just yet. Signed-off-by: Zygmunt Krynicki <me@zygoon.pl>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
const char *base_snap_name; | ||
const char *security_tag; | ||
const char *snap_instance; | ||
} sc_invocation; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat!
Currently passing data around snap-confine is not very easy because of
historically organic growth of the codebase. To simplify some of that
I'd like to introduce sc_invocation. An ad-hoc structure that captures
some of the variables that participate in the essential part of
snap-confine's logic.
The purpose of this patch is to introduce this structure, switch some of
the code to use it (as opposed to accessing the local variables) and
set the stage for a subsequent patch that moves a large chunk of code
into the two new stubs: enter_{,non_}classic_execution_environment().
This should eventually allow us to make a single call to
sc_should_use_normal_mode() and simply pass the result around via the
invocation structure. There is plenty of more refactoring that can be
done after this patch is in place.
Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com